Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Illumos 7176 Yet another hole birth issue #4950

Conversation

kernelOfTruth
Copy link
Contributor

@kernelOfTruth kernelOfTruth commented Aug 9, 2016

Illumos 7176 Yet another hole birth issue

Authored by: Paul Dagnelie [email protected]
Reviewed by: Matthew Ahrens [email protected]
Reviewed by: George Wilson [email protected]
Approved by:
Ported-by: kernelOfTruth [email protected]
OpenZFS-issue: https://www.illumos.org/issues/7176
OpenZFS-commit:

This is another bug in the long line of hole-birth related issues. In this particular case, it was discovered that a previous hole-birth fix (illumos bug 6513) did not cover as many cases as we thought
it did. While the issue worked in the case of hole-punching (writing zeroes to a large part of a file), it did not deal with truncation, and then writing beyond the new end of the file.

The problem is that dbuf_findbp will return ENOENT if the block it's trying to find is beyond the end of the file. If that happens, we assume there is no birth time, and so we lose that information when
we write out new blkptrs. We should teach dbuf_findbp to look for things that are beyond the current end, but not beyond the absolute end of the file.

Upstream bugs: DLPX-46009

@kernelOfTruth
Copy link
Contributor Author

kernelOfTruth commented Aug 9, 2016

Todo:

Commit message needs update once review & approved upstream

build needs fix, will take a look later

thanks

@kernelOfTruth kernelOfTruth changed the title OpenZFS 7176 Yet another hole birth issue Illumos 7176 Yet another hole birth issue Aug 9, 2016
@kernelOfTruth kernelOfTruth force-pushed the zfs_master_09.08.2016_openzfs-7176 branch 3 times, most recently from 12ab566 to be7db52 Compare August 17, 2016 00:11
Authored by: Paul Dagnelie <[email protected]>
Reviewed by: Matthew Ahrens [email protected]
Reviewed by: George Wilson [email protected]
Approved by:
Ported-by: kernelOfTruth <[email protected]>
OpenZFS-issue: https://www.illumos.org/issues/7176
OpenZFS-commit:

This is another bug in the long line of hole-birth related issues. In this particular case, it was discovered that a previous hole-birth fix (illumos bug 6513) did not cover as many cases as we thought
it did. While the issue worked in the case of hole-punching (writing zeroes to a large part of a file), it did not deal with truncation, and then writing beyond the new end of the file.

The problem is that dbuf_findbp will return ENOENT if the block it's trying to find is beyond the end of the file. If that happens, we assume there is no birth time, and so we lose that information when
we write out new blkptrs. We should teach dbuf_findbp to look for things that are beyond the current end, but not beyond the absolute end of the file.

Upstream bugs: DLPX-46009

Porting notes:
fix ISO C90 mixed declaration error in dbuf.c ( int nlevels, epbs; ) ;
keep previous position of the initialization
@kernelOfTruth kernelOfTruth force-pushed the zfs_master_09.08.2016_openzfs-7176 branch from be7db52 to 6267992 Compare August 18, 2016 02:10
@behlendorf
Copy link
Contributor

@bprotopopov can I add your signed-by when merging this.

@richardelling
Copy link
Contributor

@bprotopopov DN_MAX_INDBLKSHIFT to 17 is covered by PR 4800 #4800

@behlendorf
Copy link
Contributor

Right, we still need to pull that patch in after it's rebased and passes the tests.

@bprotopopov
Copy link
Contributor

Sure, np


From: Brian Behlendorf [email protected]
Sent: Thursday, August 18, 2016 12:26:02 PM
To: zfsonlinux/zfs
Cc: Boris Protopopov; Mention
Subject: Re: [zfsonlinux/zfs] Illumos 7176 Yet another hole birth issue (#4950)

@bprotopopovhttps://github.com/bprotopopov can I add your signed-by when merging this.

You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com//pull/4950#issuecomment-240778197, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ACX4uc4kUUVpR0mMTFVGaI_J5jd_06khks5qhIeagaJpZM4JghoZ.

@bprotopopov
Copy link
Contributor

Yes, that's a better approach


From: Richard Elling [email protected]
Sent: Thursday, August 18, 2016 12:31:57 PM
To: zfsonlinux/zfs
Cc: Boris Protopopov; Mention
Subject: Re: [zfsonlinux/zfs] Illumos 7176 Yet another hole birth issue (#4950)

@bprotopopovhttps://github.com/bprotopopov DN_MAX_INDBLKSHIFT to 17 is covered by PR 4800 #4800#4800

You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com//pull/4950#issuecomment-240779975, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ACX4uf5lof1FR842cUMBeJR-qAqASg9Hks5qhIj9gaJpZM4JghoZ.

@behlendorf
Copy link
Contributor

Merged as:

32d41fb OpenZFS 7176 - Yet another hole birth issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants